Conversation
|
@categulario I see that there are unfinished things here, because it's WIP, let's finish all features before creating the PR to develop. You can also cherry pick some finished changes and create a PR with that if you want, like changes in the router dependency or in the database/models, as long as the existing functionality keeps working. |
emi420
left a comment
emi420
left a comment
There was a problem hiding this comment.
Great work @categulario !! There are just a few things to be fixed before approving the PR.
- Live compatibility with new code. This feature is now broken because the map can't be created after changes on endpoints.
- Bug with tags
- Track media files ownership
- Small naming issue and missing translation
| } for map, count in maps] | ||
|
|
||
|
|
||
| @api_router.post("/map/media") |
There was a problem hiding this comment.
We should keep tracking of which file belongs to which map, like we do in uMap, so we can then remove media files when removing a map. Would make sense to add a UploadedFiles table?
We can also take advantage of this for blocking access to files when the map is private.
This is what we have in uMap:
There was a problem hiding this comment.
a table might not be necessary since the media are assigned to points which belong to a map who has an owner, thus the query to get all the media belonging to a map or a user is a single SELECT clause.
| return {"map_id": map_id, "sharing": map_obj.sharing.value} | ||
|
|
||
|
|
||
| @api_router.get("/media/{filename}", response_class=StreamingResponse) |
There was a problem hiding this comment.
Once we add the relation between files and maps, we should add access control to these files. Media files will be available only if the map is public of logged user is owner of the map. Another way to do this is to add ownership to each file.
|
All things have been reviewed and addresed |
2 participants
A few things happen here. All surrounding the idea of saving a map.
GET /mapandPOST /map) and an old one is renamed (GET /map/new). The rename allows for a more restful interface.react-router-domis replaced byreact-router.SaveButtonis renamedDownloadButtonand a newSaveButtonis created.Headercomponent is refactored to receive buttons as children, thus simplifying its logic a lot.MapListpage is created containing the table of maps that the user has created.